Refactor advection stencils to use StencilTest - Part 2
#351
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cscs-ci run default |
|
cscs-ci run default |
|
cscs-ci run default |
|
cscs-ci run default |
|
launch jenkins spack |
|
cscs-ci run default |
|
launch jenkins spack |
|
launch jenkins spack |
2 similar comments
|
launch jenkins spack |
|
launch jenkins spack |
|
launch jenkins spack |
|
cscs-ci run default |
samkellerhals
requested changes
Jan 22, 2024
There was a problem hiding this comment.
Looks good overall! I have a few comments though on refactoring some of the larger numpy test cases. In my opinion as they are now they are just too complex to grasp and would benefit from some refactoring into separate functions with a clearly defined responsibility.
Comment on lines
152
to
155
| @classmethod | ||
| def reference( | ||
| TestDivideFluxAreaListStencil01, | ||
| grid, |
There was a problem hiding this comment.
When using class methods covention is to use cls as the name of the first argument. Nonetheless I would just keep this method as a static method, and pull out all the other static methods you are calling in the reference out of the class and just define them as normal functions to keep the class smaller and make things more readable.
Comment on lines
178
to
221
| arrival_pts_1_lon_dsl = dreg_patch0_1_lon_dsl | ||
| arrival_pts_1_lat_dsl = dreg_patch0_1_lat_dsl | ||
| arrival_pts_2_lon_dsl = dreg_patch0_2_lon_dsl | ||
| arrival_pts_2_lat_dsl = dreg_patch0_2_lat_dsl | ||
| depart_pts_1_lon_dsl = dreg_patch0_4_lon_dsl # indices have to be switched so that dep 1 belongs to arr 1 (and d2->a2) | ||
| depart_pts_1_lat_dsl = dreg_patch0_4_lat_dsl | ||
| depart_pts_2_lon_dsl = dreg_patch0_3_lon_dsl | ||
| depart_pts_2_lat_dsl = dreg_patch0_3_lat_dsl | ||
|
|
||
| lvn_pos = np.where(p_vn >= 0.0, True, False) | ||
|
|
||
| # get flux area departure-line segment | ||
| fl_line_p1_lon = depart_pts_1_lon_dsl | ||
| fl_line_p1_lat = depart_pts_1_lat_dsl | ||
| fl_line_p2_lon = depart_pts_2_lon_dsl | ||
| fl_line_p2_lat = depart_pts_2_lat_dsl | ||
|
|
||
| # get triangle edge 1 (A1V3) | ||
| tri_line1_p1_lon = arrival_pts_1_lon_dsl | ||
| tri_line1_p1_lat = arrival_pts_1_lat_dsl | ||
| tri_line1_p2_lon = np.where( | ||
| lvn_pos, | ||
| np.broadcast_to(ptr_v3_lon_e[:, 0], p_vn.shape), | ||
| np.broadcast_to(ptr_v3_lon_e[:, 1], p_vn.shape), | ||
| ) | ||
| tri_line1_p2_lat = np.where( | ||
| lvn_pos, | ||
| np.broadcast_to(ptr_v3_lat_e[:, 0], p_vn.shape), | ||
| np.broadcast_to(ptr_v3_lat_e[:, 1], p_vn.shape), | ||
| ) | ||
|
|
||
| # get triangle edge 2 (A2V3) | ||
| tri_line2_p1_lon = arrival_pts_2_lon_dsl | ||
| tri_line2_p1_lat = arrival_pts_2_lat_dsl | ||
| tri_line2_p2_lon = np.where( | ||
| lvn_pos, | ||
| np.broadcast_to(ptr_v3_lon_e[:, 0], p_vn.shape), | ||
| np.broadcast_to(ptr_v3_lon_e[:, 1], p_vn.shape), | ||
| ) | ||
| tri_line2_p2_lat = np.where( | ||
| lvn_pos, | ||
| np.broadcast_to(ptr_v3_lat_e[:, 0], p_vn.shape), | ||
| np.broadcast_to(ptr_v3_lat_e[:, 1], p_vn.shape), | ||
| ) |
There was a problem hiding this comment.
I would extract this into a separate function
Comment on lines
275
to
334
| # Case 1 - patch 0 | ||
| dreg_patch0_1_lon_dsl = np.where(mask_case1, arrival_pts_1_lon_dsl, dreg_patch0_1_lon_dsl) | ||
| dreg_patch0_1_lat_dsl = np.where(mask_case1, arrival_pts_1_lat_dsl, dreg_patch0_1_lat_dsl) | ||
| dreg_patch0_2_lon_dsl = np.where( | ||
| mask_case1, | ||
| np.where(lvn_sys_pos, arrival_pts_2_lon_dsl, ps1_x), | ||
| dreg_patch0_2_lon_dsl, | ||
| ) | ||
| dreg_patch0_2_lat_dsl = np.where( | ||
| mask_case1, | ||
| np.where(lvn_sys_pos, arrival_pts_2_lat_dsl, ps1_y), | ||
| dreg_patch0_2_lat_dsl, | ||
| ) | ||
| dreg_patch0_3_lon_dsl = np.where(mask_case1, ps2_x, dreg_patch0_3_lon_dsl) | ||
| dreg_patch0_3_lat_dsl = np.where(mask_case1, ps2_y, dreg_patch0_3_lat_dsl) | ||
| dreg_patch0_4_lon_dsl = np.where( | ||
| mask_case1, | ||
| np.where(lvn_sys_pos, ps1_x, arrival_pts_2_lon_dsl), | ||
| dreg_patch0_4_lon_dsl, | ||
| ) | ||
| dreg_patch0_4_lat_dsl = np.where( | ||
| mask_case1, | ||
| np.where(lvn_sys_pos, ps1_y, arrival_pts_2_lat_dsl), | ||
| dreg_patch0_4_lat_dsl, | ||
| ) | ||
| # Case 1 - patch 1 | ||
| dreg_patch1_1_lon_vmask = np.where(mask_case1, arrival_pts_1_lon_dsl, 0.0) | ||
| dreg_patch1_1_lat_vmask = np.where(mask_case1, arrival_pts_1_lat_dsl, 0.0) | ||
| dreg_patch1_4_lon_vmask = np.where(mask_case1, arrival_pts_1_lon_dsl, 0.0) | ||
| dreg_patch1_4_lat_vmask = np.where(mask_case1, arrival_pts_1_lat_dsl, 0.0) | ||
| dreg_patch1_2_lon_vmask = np.where( | ||
| mask_case1, np.where(lvn_sys_pos, ps1_x, depart_pts_1_lon_dsl), 0.0 | ||
| ) | ||
| dreg_patch1_2_lat_vmask = np.where( | ||
| mask_case1, np.where(lvn_sys_pos, ps1_y, depart_pts_1_lat_dsl), 0.0 | ||
| ) | ||
| dreg_patch1_3_lon_vmask = np.where( | ||
| mask_case1, np.where(lvn_sys_pos, depart_pts_1_lon_dsl, ps1_x), 0.0 | ||
| ) | ||
| dreg_patch1_3_lat_vmask = np.where( | ||
| mask_case1, np.where(lvn_sys_pos, depart_pts_1_lat_dsl, ps1_y), 0.0 | ||
| ) | ||
| # Case 1 - patch 2 | ||
| dreg_patch2_1_lon_vmask = np.where(mask_case1, arrival_pts_2_lon_dsl, 0.0) | ||
| dreg_patch2_1_lat_vmask = np.where(mask_case1, arrival_pts_2_lat_dsl, 0.0) | ||
| dreg_patch2_4_lon_vmask = np.where(mask_case1, arrival_pts_2_lon_dsl, 0.0) | ||
| dreg_patch2_4_lat_vmask = np.where(mask_case1, arrival_pts_2_lat_dsl, 0.0) | ||
| dreg_patch2_2_lon_vmask = np.where( | ||
| mask_case1, np.where(lvn_sys_pos, depart_pts_2_lon_dsl, ps2_x), 0.0 | ||
| ) | ||
| dreg_patch2_2_lat_vmask = np.where( | ||
| mask_case1, np.where(lvn_sys_pos, depart_pts_2_lat_dsl, ps2_y), 0.0 | ||
| ) | ||
| dreg_patch2_3_lon_vmask = np.where( | ||
| mask_case1, np.where(lvn_sys_pos, ps2_x, depart_pts_2_lon_dsl), 0.0 | ||
| ) | ||
| dreg_patch2_3_lat_vmask = np.where( | ||
| mask_case1, np.where(lvn_sys_pos, ps2_y, depart_pts_2_lat_dsl), 0.0 | ||
| ) | ||
|
|
There was a problem hiding this comment.
Would also extract this into a separate function.
Comment on lines
339
to
363
| # Case 2a - patch 0 | ||
| dreg_patch0_1_lon_dsl = np.where(mask_case2a, arrival_pts_1_lon_dsl, dreg_patch0_1_lon_dsl) | ||
| dreg_patch0_1_lat_dsl = np.where(mask_case2a, arrival_pts_1_lat_dsl, dreg_patch0_1_lat_dsl) | ||
| dreg_patch0_2_lon_dsl = np.where( | ||
| mask_case2a, | ||
| np.where(lvn_sys_pos, arrival_pts_2_lon_dsl, ps1_x), | ||
| dreg_patch0_2_lon_dsl, | ||
| ) | ||
| dreg_patch0_2_lat_dsl = np.where( | ||
| mask_case2a, | ||
| np.where(lvn_sys_pos, arrival_pts_2_lat_dsl, ps1_y), | ||
| dreg_patch0_2_lat_dsl, | ||
| ) | ||
| dreg_patch0_3_lon_dsl = np.where(mask_case2a, depart_pts_2_lon_dsl, dreg_patch0_3_lon_dsl) | ||
| dreg_patch0_3_lat_dsl = np.where(mask_case2a, depart_pts_2_lat_dsl, dreg_patch0_3_lat_dsl) | ||
| dreg_patch0_4_lon_dsl = np.where( | ||
| mask_case2a, | ||
| np.where(lvn_sys_pos, ps1_x, arrival_pts_2_lon_dsl), | ||
| dreg_patch0_4_lon_dsl, | ||
| ) | ||
| dreg_patch0_4_lat_dsl = np.where( | ||
| mask_case2a, | ||
| np.where(lvn_sys_pos, ps1_y, arrival_pts_2_lat_dsl), | ||
| dreg_patch0_4_lat_dsl, | ||
| ) |
There was a problem hiding this comment.
Would also extract this Case 2a - patch 0 into a separate function.
Comment on lines
364
to
405
| # Case 2a - patch 1 | ||
| dreg_patch1_1_lon_vmask = np.where( | ||
| mask_case2a, arrival_pts_1_lon_dsl, dreg_patch1_1_lon_vmask | ||
| ) | ||
| dreg_patch1_1_lat_vmask = np.where( | ||
| mask_case2a, arrival_pts_1_lat_dsl, dreg_patch1_1_lat_vmask | ||
| ) | ||
| dreg_patch1_4_lon_vmask = np.where( | ||
| mask_case2a, arrival_pts_1_lon_dsl, dreg_patch1_4_lon_vmask | ||
| ) | ||
| dreg_patch1_4_lat_vmask = np.where( | ||
| mask_case2a, arrival_pts_1_lat_dsl, dreg_patch1_4_lat_vmask | ||
| ) | ||
| dreg_patch1_2_lon_vmask = np.where( | ||
| mask_case2a, | ||
| np.where(lvn_sys_pos, ps1_x, depart_pts_1_lon_dsl), | ||
| dreg_patch1_2_lon_vmask, | ||
| ) | ||
| dreg_patch1_2_lat_vmask = np.where( | ||
| mask_case2a, | ||
| np.where(lvn_sys_pos, ps1_y, depart_pts_1_lat_dsl), | ||
| dreg_patch1_2_lat_vmask, | ||
| ) | ||
| dreg_patch1_3_lon_vmask = np.where( | ||
| mask_case2a, | ||
| np.where(lvn_sys_pos, depart_pts_1_lon_dsl, ps1_x), | ||
| dreg_patch1_3_lon_vmask, | ||
| ) | ||
| dreg_patch1_3_lat_vmask = np.where( | ||
| mask_case2a, | ||
| np.where(lvn_sys_pos, depart_pts_1_lat_dsl, ps1_y), | ||
| dreg_patch1_3_lat_vmask, | ||
| ) | ||
| # Case 2a - patch 2 | ||
| dreg_patch2_1_lon_vmask = np.where(mask_case2a, 0.0, dreg_patch2_1_lon_vmask) | ||
| dreg_patch2_1_lat_vmask = np.where(mask_case2a, 0.0, dreg_patch2_1_lat_vmask) | ||
| dreg_patch2_2_lon_vmask = np.where(mask_case2a, 0.0, dreg_patch2_2_lon_vmask) | ||
| dreg_patch2_2_lat_vmask = np.where(mask_case2a, 0.0, dreg_patch2_2_lat_vmask) | ||
| dreg_patch2_3_lon_vmask = np.where(mask_case2a, 0.0, dreg_patch2_3_lon_vmask) | ||
| dreg_patch2_3_lat_vmask = np.where(mask_case2a, 0.0, dreg_patch2_3_lat_vmask) | ||
| dreg_patch2_4_lon_vmask = np.where(mask_case2a, 0.0, dreg_patch2_4_lon_vmask) | ||
| dreg_patch2_4_lat_vmask = np.where(mask_case2a, 0.0, dreg_patch2_4_lat_vmask) |
There was a problem hiding this comment.
Would also extract this Case 2a - patch 1 into a separate function.
Comment on lines
407
to
435
| # -------------------------------------------------- Case 2b | ||
| mask_case2b = np.logical_and.reduce( | ||
| [lintersect_line2, np.logical_not(lintersect_line1), famask_bool] | ||
| ) | ||
| # Case 2b - patch 0 | ||
| dreg_patch0_1_lon_dsl = np.where(mask_case2b, arrival_pts_1_lon_dsl, dreg_patch0_1_lon_dsl) | ||
| dreg_patch0_1_lat_dsl = np.where(mask_case2b, arrival_pts_1_lat_dsl, dreg_patch0_1_lat_dsl) | ||
| dreg_patch0_2_lon_dsl = np.where( | ||
| mask_case2b, | ||
| np.where(lvn_sys_pos, arrival_pts_2_lon_dsl, depart_pts_1_lon_dsl), | ||
| dreg_patch0_2_lon_dsl, | ||
| ) | ||
| dreg_patch0_2_lat_dsl = np.where( | ||
| mask_case2b, | ||
| np.where(lvn_sys_pos, arrival_pts_2_lat_dsl, depart_pts_1_lat_dsl), | ||
| dreg_patch0_2_lat_dsl, | ||
| ) | ||
| dreg_patch0_3_lon_dsl = np.where(mask_case2b, ps2_x, dreg_patch0_3_lon_dsl) | ||
| dreg_patch0_3_lat_dsl = np.where(mask_case2b, ps2_y, dreg_patch0_3_lat_dsl) | ||
| dreg_patch0_4_lon_dsl = np.where( | ||
| mask_case2b, | ||
| np.where(lvn_sys_pos, depart_pts_1_lon_dsl, arrival_pts_2_lon_dsl), | ||
| dreg_patch0_4_lon_dsl, | ||
| ) | ||
| dreg_patch0_4_lat_dsl = np.where( | ||
| mask_case2b, | ||
| np.where(lvn_sys_pos, depart_pts_1_lat_dsl, arrival_pts_2_lat_dsl), | ||
| dreg_patch0_4_lat_dsl, | ||
| ) |
There was a problem hiding this comment.
Would also extract this Case 2b into a separate function.
Comment on lines
436
to
477
| # Case 2b - patch 1 | ||
| dreg_patch1_1_lon_vmask = np.where(mask_case2b, 0.0, dreg_patch1_1_lon_vmask) | ||
| dreg_patch1_1_lat_vmask = np.where(mask_case2b, 0.0, dreg_patch1_1_lat_vmask) | ||
| dreg_patch1_2_lon_vmask = np.where(mask_case2b, 0.0, dreg_patch1_2_lon_vmask) | ||
| dreg_patch1_2_lat_vmask = np.where(mask_case2b, 0.0, dreg_patch1_2_lat_vmask) | ||
| dreg_patch1_3_lon_vmask = np.where(mask_case2b, 0.0, dreg_patch1_3_lon_vmask) | ||
| dreg_patch1_3_lat_vmask = np.where(mask_case2b, 0.0, dreg_patch1_3_lat_vmask) | ||
| dreg_patch1_4_lon_vmask = np.where(mask_case2b, 0.0, dreg_patch1_4_lon_vmask) | ||
| dreg_patch1_4_lat_vmask = np.where(mask_case2b, 0.0, dreg_patch1_4_lat_vmask) | ||
| # Case 2b - patch 2 | ||
| dreg_patch2_1_lon_vmask = np.where( | ||
| mask_case2b, arrival_pts_2_lon_dsl, dreg_patch2_1_lon_vmask | ||
| ) | ||
| dreg_patch2_1_lat_vmask = np.where( | ||
| mask_case2b, arrival_pts_2_lat_dsl, dreg_patch2_1_lat_vmask | ||
| ) | ||
| dreg_patch2_4_lon_vmask = np.where( | ||
| mask_case2b, arrival_pts_2_lon_dsl, dreg_patch2_4_lon_vmask | ||
| ) | ||
| dreg_patch2_4_lat_vmask = np.where( | ||
| mask_case2b, arrival_pts_2_lat_dsl, dreg_patch2_4_lat_vmask | ||
| ) | ||
| dreg_patch2_2_lon_vmask = np.where( | ||
| mask_case2b, | ||
| np.where(lvn_sys_pos, depart_pts_2_lon_dsl, ps2_x), | ||
| dreg_patch2_2_lon_vmask, | ||
| ) | ||
| dreg_patch2_2_lat_vmask = np.where( | ||
| mask_case2b, | ||
| np.where(lvn_sys_pos, depart_pts_2_lat_dsl, ps2_y), | ||
| dreg_patch2_2_lat_vmask, | ||
| ) | ||
| dreg_patch2_3_lon_vmask = np.where( | ||
| mask_case2b, | ||
| np.where(lvn_sys_pos, ps2_x, depart_pts_2_lon_dsl), | ||
| dreg_patch2_3_lon_vmask, | ||
| ) | ||
| dreg_patch2_3_lat_vmask = np.where( | ||
| mask_case2b, | ||
| np.where(lvn_sys_pos, ps2_y, depart_pts_2_lat_dsl), | ||
| dreg_patch2_3_lat_vmask, | ||
| ) |
There was a problem hiding this comment.
Would also extract this Case 2b - patch 1 into a separate function.
Comment on lines
513
to
699
| tri_line2_p1_lat, | ||
| tri_line2_p2_lon, | ||
| tri_line2_p2_lat, | ||
| ) | ||
| # Case 3b - patch 0 | ||
| dreg_patch0_1_lon_dsl = np.where(mask_case3b, arrival_pts_1_lon_dsl, dreg_patch0_1_lon_dsl) | ||
| dreg_patch0_1_lat_dsl = np.where(mask_case3b, arrival_pts_1_lat_dsl, dreg_patch0_1_lat_dsl) | ||
| dreg_patch0_4_lon_dsl = np.where(mask_case3b, arrival_pts_1_lon_dsl, dreg_patch0_4_lon_dsl) | ||
| dreg_patch0_4_lat_dsl = np.where(mask_case3b, arrival_pts_1_lat_dsl, dreg_patch0_4_lat_dsl) | ||
| dreg_patch0_2_lon_dsl = np.where( | ||
| mask_case3b, | ||
| np.where(lvn_sys_pos, arrival_pts_2_lon_dsl, pi2_x), | ||
| dreg_patch0_2_lon_dsl, | ||
| ) | ||
| dreg_patch0_2_lat_dsl = np.where( | ||
| mask_case3b, | ||
| np.where(lvn_sys_pos, arrival_pts_2_lat_dsl, pi2_y), | ||
| dreg_patch0_2_lat_dsl, | ||
| ) | ||
| dreg_patch0_3_lon_dsl = np.where( | ||
| mask_case3b, | ||
| np.where(lvn_sys_pos, pi2_x, arrival_pts_2_lon_dsl), | ||
| dreg_patch0_3_lon_dsl, | ||
| ) | ||
| dreg_patch0_3_lat_dsl = np.where( | ||
| mask_case3b, | ||
| np.where(lvn_sys_pos, pi2_y, arrival_pts_2_lat_dsl), | ||
| dreg_patch0_3_lat_dsl, | ||
| ) | ||
| # Case 3b - patch 1 | ||
| dreg_patch1_1_lon_vmask = np.where(mask_case3b, 0.0, dreg_patch1_1_lon_vmask) | ||
| dreg_patch1_1_lat_vmask = np.where(mask_case3b, 0.0, dreg_patch1_1_lat_vmask) | ||
| dreg_patch1_2_lon_vmask = np.where(mask_case3b, 0.0, dreg_patch1_2_lon_vmask) | ||
| dreg_patch1_2_lat_vmask = np.where(mask_case3b, 0.0, dreg_patch1_2_lat_vmask) | ||
| dreg_patch1_3_lon_vmask = np.where(mask_case3b, 0.0, dreg_patch1_3_lon_vmask) | ||
| dreg_patch1_3_lat_vmask = np.where(mask_case3b, 0.0, dreg_patch1_3_lat_vmask) | ||
| dreg_patch1_4_lon_vmask = np.where(mask_case3b, 0.0, dreg_patch1_4_lon_vmask) | ||
| dreg_patch1_4_lat_vmask = np.where(mask_case3b, 0.0, dreg_patch1_4_lat_vmask) | ||
| # Case 3b - patch 2 | ||
| dreg_patch2_1_lon_vmask = np.where( | ||
| mask_case3b, arrival_pts_2_lon_dsl, dreg_patch2_1_lon_vmask | ||
| ) | ||
| dreg_patch2_1_lat_vmask = np.where( | ||
| mask_case3b, arrival_pts_2_lat_dsl, dreg_patch2_1_lat_vmask | ||
| ) | ||
| dreg_patch2_2_lon_vmask = np.where( | ||
| mask_case3b, | ||
| np.where(lvn_sys_pos, depart_pts_2_lon_dsl, pi2_x), | ||
| dreg_patch2_2_lon_vmask, | ||
| ) | ||
| dreg_patch2_2_lat_vmask = np.where( | ||
| mask_case3b, | ||
| np.where(lvn_sys_pos, depart_pts_2_lat_dsl, pi2_y), | ||
| dreg_patch2_2_lat_vmask, | ||
| ) | ||
| dreg_patch2_3_lon_vmask = np.where( | ||
| mask_case3b, depart_pts_1_lon_dsl, dreg_patch2_3_lon_vmask | ||
| ) | ||
| dreg_patch2_3_lat_vmask = np.where( | ||
| mask_case3b, depart_pts_1_lat_dsl, dreg_patch2_3_lat_vmask | ||
| ) | ||
| dreg_patch2_4_lon_vmask = np.where( | ||
| mask_case3b, | ||
| np.where(lvn_sys_pos, pi2_x, depart_pts_2_lon_dsl), | ||
| dreg_patch2_4_lon_vmask, | ||
| ) | ||
| dreg_patch2_4_lat_vmask = np.where( | ||
| mask_case3b, | ||
| np.where(lvn_sys_pos, pi2_y, depart_pts_2_lat_dsl), | ||
| dreg_patch2_4_lat_vmask, | ||
| ) | ||
|
|
||
| # --------------------------------------------- Case 4 | ||
| # NB: Next line acts as the "ELSE IF", indices that already previously matched one of the above conditions | ||
| # can't be overwritten by this new condition. | ||
| indices_previously_matched = np.logical_or.reduce( | ||
| [mask_case3b, mask_case3a, mask_case2b, mask_case2a, mask_case1] | ||
| ) | ||
| # mask_case4 = (abs(p_vn) < 0.1) & famask_bool & (not indices_previously_matched) we insert also the error indices | ||
| mask_case4 = np.logical_and.reduce( | ||
| [famask_bool, np.logical_not(indices_previously_matched)] | ||
| ) | ||
| # Case 4 - patch 0 - no change | ||
| # Case 4 - patch 1 | ||
| dreg_patch1_1_lon_vmask = np.where(mask_case4, 0.0, dreg_patch1_1_lon_vmask) | ||
| dreg_patch1_1_lat_vmask = np.where(mask_case4, 0.0, dreg_patch1_1_lat_vmask) | ||
| dreg_patch1_2_lon_vmask = np.where(mask_case4, 0.0, dreg_patch1_2_lon_vmask) | ||
| dreg_patch1_2_lat_vmask = np.where(mask_case4, 0.0, dreg_patch1_2_lat_vmask) | ||
| dreg_patch1_3_lon_vmask = np.where(mask_case4, 0.0, dreg_patch1_3_lon_vmask) | ||
| dreg_patch1_3_lat_vmask = np.where(mask_case4, 0.0, dreg_patch1_3_lat_vmask) | ||
| dreg_patch1_4_lon_vmask = np.where(mask_case4, 0.0, dreg_patch1_4_lon_vmask) | ||
| dreg_patch1_4_lat_vmask = np.where(mask_case4, 0.0, dreg_patch1_4_lat_vmask) | ||
| # Case 4 - patch 2 | ||
| dreg_patch2_1_lon_vmask = np.where(mask_case4, 0.0, dreg_patch2_1_lon_vmask) | ||
| dreg_patch2_1_lat_vmask = np.where(mask_case4, 0.0, dreg_patch2_1_lat_vmask) | ||
| dreg_patch2_2_lon_vmask = np.where(mask_case4, 0.0, dreg_patch2_2_lon_vmask) | ||
| dreg_patch2_2_lat_vmask = np.where(mask_case4, 0.0, dreg_patch2_2_lat_vmask) | ||
| dreg_patch2_3_lon_vmask = np.where(mask_case4, 0.0, dreg_patch2_3_lon_vmask) | ||
| dreg_patch2_3_lat_vmask = np.where(mask_case4, 0.0, dreg_patch2_3_lat_vmask) | ||
| dreg_patch2_4_lon_vmask = np.where(mask_case4, 0.0, dreg_patch2_4_lon_vmask) |
There was a problem hiding this comment.
Please try to extract the remaining cases into separate functions. Ideally the reference function here just calls the individual case functions. This will make the code better organised, more readable and easier to debug in the future.
| z_eta_3_4 = 1.0 + zeta_3 | ||
| z_eta_4_4 = 1.0 + zeta_4 | ||
| @staticmethod | ||
| def reference( |
There was a problem hiding this comment.
The code in this function could maybe also be split into three different calls. One for computing wgt, one for computing z_gauss_points and one for computing vector_sums
| z_wgt_2 = 0.0625 * wgt_zeta_1 * wgt_eta_2 | ||
| z_wgt_3 = 0.0625 * wgt_zeta_2 * wgt_eta_1 | ||
| z_wgt_4 = 0.0625 * wgt_zeta_2 * wgt_eta_2 | ||
| class TestPrepGaussQuadratureCStencil(StencilTest): |
There was a problem hiding this comment.
Could we again do the split here with quad vector sums, wgt, and z_gauss points for readability? Whilst not necessary now I can also start to see some repetition in logic between stencils, I think at some point in the future we could then also share that between stencils but its not worth the extra effort now I think.
|
launch jenkins spack |
|
launch jenkins spack |
|
cscs-ci run default |
|
Mandatory Tests Please make sure you run these tests via comment before you merge!
Optional Tests To run benchmarks you can use:
In case your change might affect downstream icon-exclaim, please consider running
For more detailed information please look at CI in the EXCLAIM universe. |
|
launch jenkins spack |
|
cscs-ci run default |
|
launch jenkins spack |
|
cscs-ci run default |
samkellerhals
approved these changes
Jan 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR:
StencilTestclass. First half is in this PR.C2CECECoffset for sparse fields inSimpleGridandIconGrid.GridTypein all stencils (to be confirmed if needed in all of them)